add sandbox PoC#99
Conversation
|
|
Hey @amir-at-bunny, this looks awesome! Can you run |
|
@jamie-at-bunny prompt added to the sandbox name as discussed. |
|
@amir-at-bunny is this PR "Ready for review"? Let's get it in 🚀 |
|
To use Codex here, create a Codex account and connect to github. |
|
@greptile-apps review |
Greptile SummaryThis PR introduces a
Confidence Score: 3/5Several concrete defects need fixing before this reaches users: the SSL/https mismatch will produce broken URLs, the unquoted cwd will silently misbehave on any path with a space, and the missing JSON output branch will break any scripted consumer. The create command never checks output === 'json' so piping its output into a script always yields human-readable text. The url add/list commands create a plain-HTTP CDN endpoint but hand users an https:// URL that won't work. The exec remote command builds a raw shell string from the user-supplied --cwd without quoting, so any path containing a space fails silently. The imagePullPolicy choice means deployed sandboxes will run stale images indefinitely once pulled. These are present, concrete misbehaviours on the changed code paths. packages/cli/src/commands/sandbox/create.ts, packages/cli/src/commands/sandbox/exec.ts, packages/cli/src/commands/sandbox/url/add.ts, packages/cli/src/commands/sandbox/url/list.ts
|
| Filename | Overview |
|---|---|
| packages/cli/src/commands/sandbox/create.ts | Creates sandboxes via Magic Containers API; no JSON output path, imagePullPolicy ifNotPresent with latest tag silently pins stale images, and inline App type + as any casts bypass generated schema types. |
| packages/cli/src/commands/sandbox/exec.ts | Runs SSH commands in sandboxes; unquoted cwd in the remote command string breaks paths with spaces and allows shell injection via the --cwd flag. |
| packages/cli/src/commands/sandbox/url/add.ts | Exposes container ports as CDN endpoints; creates the endpoint with isSslEnabled:false but displays the resulting URL as https://, producing broken links. |
| packages/cli/src/commands/sandbox/ssh-exec.ts | Shared SSH argument builder; disables host-key verification on every connection exposing connections to MITM over the public anycast endpoint. |
| packages/cli/src/commands/sandbox/url/list.ts | Lists CDN endpoints for a sandbox; filters built-in endpoints correctly but mirrors the https:// prefix bug from url/add.ts. |
| packages/cli/src/commands/sandbox/delete.ts | Deletes sandbox and its MC app with optional --force; confirmation flow and local config cleanup look correct. |
| packages/cli/src/commands/sandbox/list.ts | Lists sandboxes from local config file; table and JSON output both handled correctly. |
| packages/cli/src/commands/sandbox/ssh.ts | Opens an interactive SSH session into a sandbox; straightforward implementation, relies on ssh-exec.ts for argument construction. |
| packages/cli/src/config/schema.ts | Adds SandboxRecord Zod schema and extends ConfigFileSchema; schema shape is correct and backward compatible. |
| packages/cli/src/config/index.ts | Adds getSandbox/setSandbox/deleteSandbox helpers; read-modify-write pattern matches existing setProfile convention. |
| sandbox/Dockerfile | Ubuntu 24.04 image with Node.js LTS, Bun, Claude Code, and openssh-server on port 8023; root password auth and PermitRootLogin are intentional PoC choices. |
| sandbox/entrypoint.sh | Sets root password to AGENT_TOKEN at container start and launches sshd; simple and correct for the intended design. |
| .github/workflows/sandbox-agent.yml | CI workflow to build and push sandbox Docker image to GHCR on pushes to main affecting sandbox/**; only pushes latest tag with no digest pinning. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant CLI
participant MC_API as Magic Containers API
participant Sandbox as Sandbox Container
User->>CLI: bunny sandbox create [name] [--region]
CLI->>MC_API: POST /apps (image, volumes, SSH endpoint)
MC_API-->>CLI: "app { id }"
loop Poll until anycast SSH endpoint assigned
CLI->>MC_API: "GET /apps/{appId}"
MC_API-->>CLI: "app { status, endpoints[] }"
end
loop Probe TCP until SSH port accepts connections
CLI-->>Sandbox: TCP connect :8023
end
CLI->>CLI: "setSandbox(name, { app_id, agent_token, ssh_host })"
CLI-->>User: Sandbox ready (app_id, ssh_host)
User->>CLI: bunny sandbox exec [name] [cmd] [--cwd]
CLI->>CLI: getSandbox(name) → record
CLI->>Sandbox: "sshpass -p token ssh root@host cmd"
Sandbox-->>User: stdout/stderr (exit code propagated)
User->>CLI: bunny sandbox url add [name] [port] [--label]
CLI->>MC_API: "POST /apps/{appId}/containers/{id}/endpoints"
loop Poll until publicHost assigned
CLI->>MC_API: "GET /apps/{appId}/endpoints"
MC_API-->>CLI: "endpoint { publicHost }"
end
CLI-->>User: https://publicHost
User->>CLI: bunny sandbox delete [name] [--force]
CLI->>MC_API: "DELETE /apps/{appId}"
CLI->>CLI: deleteSandbox(name)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant CLI
participant MC_API as Magic Containers API
participant Sandbox as Sandbox Container
User->>CLI: bunny sandbox create [name] [--region]
CLI->>MC_API: POST /apps (image, volumes, SSH endpoint)
MC_API-->>CLI: "app { id }"
loop Poll until anycast SSH endpoint assigned
CLI->>MC_API: "GET /apps/{appId}"
MC_API-->>CLI: "app { status, endpoints[] }"
end
loop Probe TCP until SSH port accepts connections
CLI-->>Sandbox: TCP connect :8023
end
CLI->>CLI: "setSandbox(name, { app_id, agent_token, ssh_host })"
CLI-->>User: Sandbox ready (app_id, ssh_host)
User->>CLI: bunny sandbox exec [name] [cmd] [--cwd]
CLI->>CLI: getSandbox(name) → record
CLI->>Sandbox: "sshpass -p token ssh root@host cmd"
Sandbox-->>User: stdout/stderr (exit code propagated)
User->>CLI: bunny sandbox url add [name] [port] [--label]
CLI->>MC_API: "POST /apps/{appId}/containers/{id}/endpoints"
loop Poll until publicHost assigned
CLI->>MC_API: "GET /apps/{appId}/endpoints"
MC_API-->>CLI: "endpoint { publicHost }"
end
CLI-->>User: https://publicHost
User->>CLI: bunny sandbox delete [name] [--force]
CLI->>MC_API: "DELETE /apps/{appId}"
CLI->>CLI: deleteSandbox(name)
Comments Outside Diff (4)
-
packages/cli/src/commands/sandbox/url/add.ts, line 1004-1006 (link)SSL disabled but URL shown as
https://The endpoint is created with
isSslEnabled: false, but the URL printed to the user (and also inurl/list.ts) is always prefixed withhttps://. If the CDN endpoint genuinely has no SSL, every link handed to the user will fail in a browser with a TLS error. Either enable SSL at creation time or prefix the URL withhttp://to match the actual endpoint configuration. -
packages/cli/src/commands/sandbox/create.ts, line 574-588 (link)sandbox createhas no JSON output pathThe AGENTS.md convention requires every command that returns data to support
--output json. The handler receivesoutputbut never checks foroutput === "json"— callers using--output jsonreceive human-readablelogger.loglines, breaking any scripted or agent usage. A JSON branch should return a structured object (e.g.{ name, app_id, ssh_host }) before the plain-text log block. -
packages/cli/src/commands/sandbox/ssh-exec.ts, line 24-40 (link)Host key verification fully disabled
StrictHostKeyChecking=nocombined withUserKnownHostsFile=/dev/nullmeans the SSH client accepts any host key without verification on every connection, making the connection silently vulnerable to a machine-in-the-middle attack over the public anycast endpoint. Consider-o StrictHostKeyChecking=accept-newso the key is pinned on first connect and validated on subsequent ones. -
packages/cli/src/commands/sandbox/create.ts, line 359-375 (link)Inline
Apptype instead of generated schema typesAGENTS.md and CLAUDE.md both require using
Pick<components["schemas"]["TypeName"], ...>from the generated Magic Containers spec instead of hand-rolled record types. The inlineApptype here, and the(client as any).POSTcast for the create call, hide type errors. Consider defining aCustomPathsentry for the undocumentedPOST /appsendpoint following the existing pattern incore-client.ts.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "add prompt to sandbox names" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ); | ||
| } | ||
|
|
||
| const remoteCmd = `cd ${cwd} && ${command.join(" ")}`; |
There was a problem hiding this comment.
cwd breaks paths with spaces and is susceptible to shell injection
cwd is user-supplied via --cwd. Any path containing spaces (e.g. --cwd "/my project") will silently fail the cd, and a value like --cwd '/tmp; rm -rf /workplace' will run extra shell commands in the sandbox. Quoting the path with JSON.stringify (which produces a double-quoted, properly escaped shell string) prevents both issues.
| const remoteCmd = `cd ${cwd} && ${command.join(" ")}`; | |
| const remoteCmd = `cd ${JSON.stringify(cwd)} && ${command.join(" ")}`; |
| imageNamespace: IMAGE_NAMESPACE, | ||
| imageName: IMAGE_NAME, | ||
| imageTag: IMAGE_TAG, | ||
| imagePullPolicy: "ifNotPresent", |
There was a problem hiding this comment.
ifNotPresent with a latest tag means sandbox images are never updated
When the image policy is ifNotPresent and the tag is latest, the runtime pulls the image once and then never re-pulls it, even if a new latest is pushed (e.g. a security patch to Claude Code or Bun). Using always ensures each sandbox creation picks up the most recent build.
| imagePullPolicy: "ifNotPresent", | |
| imagePullPolicy: "always", |
yep. thanks i'll take a look at the review in a bit. |
2 participants
No description provided.